Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds filter for failed to replace env in config erros #10237

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

sachin-sandhu
Copy link
Contributor

@sachin-sandhu sachin-sandhu commented Jul 18, 2024

What are you trying to accomplish?

Ecosystem : NPM and YARN
Exception Count : 18.3k (Since April 2024)

Preface: Resolving issues related with Dependabot::SharedHelpers::HelperSubprocessFailed issues

Issue: Dependabot will throw uknown_error in case a configuration file is missing an env token. Example
"//registry.npmjs.org/:_authToken" ${NPM_TOKEN}. Usually env vars are provided through .env file with the project repo or through a CI/CD pipeline. Following is variation of errors that is returned from native helper:

Failed to replace env in config: ${GITHUB_TOKEN}
Failed to replace env in config: ${ARTIFACTORY_READ_USER}
Failed to replace env in config: ${FAST_HTTPAUTH}
Failed to replace env in config: ${NPM_TOKEN}

Current error log:

+-------------------------------+
| Dependencies failed to update |
+---------------+---------------+
| js | unknown_error |
+---------------+---------------+

Fix: To capture the response returned from native helper and raise new exception failed_to_replace_env exception instead.

+---------------------------------------------------+
| Dependencies failed to update |
+-----------+---------------------------------------+
| io | failed_to_replace_env |
+-----------+---------------------------------------+

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@sachin-sandhu sachin-sandhu self-assigned this Jul 18, 2024
@sachin-sandhu sachin-sandhu marked this pull request as ready for review July 18, 2024 03:45
@sachin-sandhu sachin-sandhu requested a review from a team as a code owner July 18, 2024 03:45
Copy link
Member

@abdulapopoola abdulapopoola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving provided the PR comments are addressed

Copy link
Contributor

@kbukum1 kbukum1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After extending TypedDependabotError and use it accordingly as mentioned in the comment everything seems great. Nice work.


sig { params(error_message: String).returns(String) }
def self.extract_var(error_message)
match_data = error_message.match(ENV_VAR_NOT_RESOLVABLE)

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on a
library input
may run slow on strings starting with 'Failed to replace env in config: ${' and with many repetitions of 'Failed to replace env in config: ${a'.
npm_and_yarn/lib/dependabot/npm_and_yarn.rb Dismissed Show dismissed Hide dismissed
@sachin-sandhu sachin-sandhu force-pushed the fixes-failed-to-replace-env-in-config branch from 6ad4364 to 47f6791 Compare July 19, 2024 01:36
npm_and_yarn/lib/dependabot/npm_and_yarn.rb Dismissed Show dismissed Hide dismissed
@sachin-sandhu sachin-sandhu merged commit 8fc1df4 into main Jul 19, 2024
159 checks passed
@sachin-sandhu sachin-sandhu deleted the fixes-failed-to-replace-env-in-config branch July 19, 2024 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants